Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix MacOS & Windows compilation #143

Merged
merged 1 commit into from
Mar 28, 2024
Merged

Fix MacOS & Windows compilation #143

merged 1 commit into from
Mar 28, 2024

Conversation

heavenboy8
Copy link
Contributor

The SEV library was compiling on MacOS & Windows previously. Recent commits have broken that. This PR fixes it.
Also add MacOS & Windows runners in the CI workflow to keep MacOS & Windows support in the future

@heavenboy8
Copy link
Contributor Author

@larrydewey @DGonzalezVillal
Thanks in advance ;)

@larrydewey
Copy link
Contributor

@heavenboy8 These are some good changes. @DGonzalezVillal is working on a PR which will remove the endpoints which are causing issues for CI, so when that gets finished, I will leave another comment on here, you can rebase, and we will review this and pull in your changes.

@larrydewey
Copy link
Contributor

@heavenboy8 will you rebase and we will review and merge?

@tylerfanelli
Copy link
Member

Can the commits be squashed before we merge?

@DGonzalezVillal
Copy link
Member

DGonzalezVillal commented Feb 22, 2024

@heavenboy8 we are planning on having a release soon, to have this on release, please rebase and squash commits!

@ThibsG
Copy link

ThibsG commented Mar 8, 2024

friendly ping @larrydewey ,@tylerfanelli and @DGonzalezVillal, I did the rebase and squashed the commits so it's ready to be reviewed. Sorry for the delay.
The Sign-off check fails, I guess it's because I pushed the commits in place of @heavenboy8 , am I right? How to solve this?

@DGonzalezVillal
Copy link
Member

friendly ping @larrydewey ,@tylerfanelli and @DGonzalezVillal, I did the rebase and squashed the commits so it's ready to be reviewed. Sorry for the delay. The Sign-off check fails, I guess it's because I pushed the commits in place of @heavenboy8 , am I right? How to solve this?

@ThibsG if you git commit -s then push, does that fix it?

@tylerfanelli
Copy link
Member

git commit --amend -s

If that doesn't work, you can also add it manually to the git commit message, git commit --amend and then write

Signed-off-by: ...

Signed-off-by: Thibaud Genty <thibaud.genty@cosmian.com>
@ThibsG
Copy link

ThibsG commented Mar 8, 2024

Thanks! I didn't know -s, only the -S option 😉

Copy link
Member

@tylerfanelli tylerfanelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @larrydewey what do you think?

Copy link
Member

@DGonzalezVillal DGonzalezVillal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me. In the runners i did notice mac and linux were listed for openssl testing and windows was only listed for no openssl. Does Windows not support openssl? It seems most of it wouldn't be compiled outside of linux anyway

Comment on lines +17 to +19
runner:
- ubuntu-latest
- macos-12
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does Windows not support Openssl?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@larrydewey larrydewey merged commit fb28b1d into virtee:main Mar 28, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants